-
Notifications
You must be signed in to change notification settings - Fork 391
refactor(benchmark): repricing filter logic #1810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: forks/osaka
Are you sure you want to change the base?
refactor(benchmark): repricing filter logic #1810
Conversation
| if self.fixed_opcode_count is not None and self.code_generator is None: | ||
| pytest.skip( | ||
| "Cannot run fixed opcode count tests without a code generator" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the fixed opcode count feature is supported only by benchmark tests that use the code generator. If a test does not support this feature, it should be skipped.
There are two possible ways to filter such tests:
We could skip them during collection via pytest_collection_modifyitems, but this is more complex because it requires determining whether each test is a benchmark test and whether it uses the code generator.
Therefore, I chose to skip the unsupported tests directly in benchmark test wrapper instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine, it's a very fast check so the difference in time should be negligible. 👍
2110a2c to
b54883f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1810 +/- ##
============================================
Coverage 87.31% 87.31%
============================================
Files 541 541
Lines 32832 32832
Branches 3015 3015
============================================
Hits 28668 28668
Misses 3557 3557
Partials 607 607
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
b54883f to
2aa61b7
Compare
2aa61b7 to
3ab4de0
Compare
Carsons-Eels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know the EEST code as well as EELS, but I think this looks good.
3ab4de0 to
b5047d6
Compare
🗒️ Description
Supported Command:
fill&executeModes:
-m benchmark(undercompute/folder)-m stateful(understateful/folder)Flags
--gas-benchmark-values(specify the block gas limit)--fixed-opcode-count(specify the opcode count)Options
-m repricing(run only the subset of benchmark tests for quick testing)Issue Description
When running benchmark tests with the
--fixed-opcode-countflag, the current behavior incorrectly restricts execution to only the benchmark cases marked with the repricing marker.However,
repricingis meant to be an independent option that limits execution to a subset of the benchmark suite.Using
--fixed-opcode-countshould not imply repricing mode.Fix Applied in This PR
This PR adjusts the logic so that:
--fixed-opcode-countexecutes the full benchmark suite: Four passes are executed (includingcontract_balancewith values 0 and 1):Add a sanity check CI workflow for fixed opcode count feature.
🔗 Related Issues or PRs
None
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture